Skip to content

Conversation

@benma
Copy link
Contributor

@benma benma commented Oct 16, 2025

No description provided.

@benma benma requested review from bznein and shonsirsha October 16, 2025 15:05
alertUser(t('device.firmwareUpgradeRequired'));
return;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in Pocket we use this

<Dialog title={t('upgradeFirmware.title')} open={fwRequiredDialog}>
{t('device.firmwareUpgradeRequired')}
<DialogButtons>
<Button
primary
onClick={() => {
setFirmwareUpdateDialogOpen(true);
navigate(`/settings/device-settings/${deviceIDs[0] as string}`);
}}>
{t('upgradeFirmware.button')}
</Button>
<Button
secondary
onClick={() => {
setFwRequiredDialog(false);
navigate(-1);
}}>
{t('dialog.cancel')}
</Button>
</DialogButtons>
</Dialog>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Will ask my agent to extract this into a resuable component later 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thisconnect done, PTAL.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with 9.18.0 and tried to find why it didn't show. But reading the code the FW upgrade dialog should only shows when handleSend is called. Could you change to it shows earlier?

I suggest either when the user clicks the dropdown or after choosing an option from the dropdown.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree - I think it'd be nice to show it earlier, before the user presses the send btn

Copy link
Contributor Author

@benma benma Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done now. I accidentally amended it into the previous commit, and it was too difficult to undo it, sorry!

This PR is now also building on top of #3653 (rebased)

PTAL

Copy link
Collaborator

@bznein bznein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backend utACK with a minor comment (and I think you probably need to regenerate the mock file)

handlers.log.WithField("requested", rootFingerprintHex).Warn("features requested for non-connected keystore")
return response{
Success: false,
ErrorMessage: "keystore not connected",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] maybe something clearer that shows the difference between this case and the one where connectedKeystore is nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@benma benma force-pushed the send-to-self-version-check branch 2 times, most recently from 8ca7859 to 0fdee13 Compare October 16, 2025 18:39
@benma benma force-pushed the send-to-self-version-check branch from 0fdee13 to 55f4905 Compare October 28, 2025 11:42
benma's agent added 2 commits October 28, 2025 16:35
And move the check to when selecting the account already, not when
clicking Review.

`deviceIDs` is re-fetched so we don't have to pass it everywhere.
@benma benma force-pushed the send-to-self-version-check branch from 55f4905 to cea342e Compare October 28, 2025 15:35
Copy link
Collaborator

@thisconnect thisconnect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested (webdev, old fw, altcoins) LGTM

but I noticed an unrelated "bug" that is already on staging-send-to-self-dropdown

When I reject the signing on the device and the "Edit transaction" and "Review" again I get: `Failed to sign transaction: Invalid Psbt due to duplicate key

Screenshot 2025-10-29 at 09 16 59

<AppRouter
accounts={accounts}
activeAccounts={activeAccounts}
deviceIDs={deviceIDs}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

@benma benma merged commit f0dce1b into BitBoxSwiss:staging-send-to-self-dropdown Oct 29, 2025
10 of 11 checks passed
@benma benma deleted the send-to-self-version-check branch October 29, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants